Repository navigation
Conversation
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This range contains a large multi-area integration rather than a bounded queue-navigation change, including new user workflows, security-sensitive provider code, deployment changes, and substantial server/desktop/mobile behavior changes. It also changes product defaults and adds static-analysis suppressions, while unresolved findings report critical authorization and orchestration risks. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
|
Macroscope has since reviewed this pull request. An earlier review was skipped by a cost limit; a review has now completed, so that notice no longer applies. |
02b5187 to
1004723
Compare
1bd44f2 to
3b9c885
Compare
1004723 to
03c1a7a
Compare
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
6448cfe to
1563a64
Compare
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
#11566) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ed restarts (#11565) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ation fails (#11557) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ive (#13517) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…156.1 (#13533) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…13535) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…from source (#13571) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
#13570) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…inished (#13557) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…#13562) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
fe4f6ad to
87c67bd
Compare
03c1a7a to
ce61b28
Compare
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ce61b28 to
024abdc
Compare
The native queue sheet was a flat one-line list with a small resume pill. Rows now sit in a grouped card with two-line previews, a status line (up next, position, attachment count, or editing), a larger image thumbnail, and a quieter Steer pill. A held queue shows a paused banner with an inline Resume, and a hint explains tap, hold, and swipe. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| (cause) => new ProjectOperationError({ operation: "list-threads", projectId, cause }), | ||
| ), | ||
| ); | ||
| const projectThreads = [...snapshot.threads, ...snapshot.archivedThreads].filter( |
There was a problem hiding this comment.
🟠 High project/ProjectService.ts:396
deleteProject can soft-delete a project while leaving a newly created live V2 thread attached to it, along with that thread’s run/provider-session resources. Because projectThreads comes from the one-time shell snapshot at line 389, a thread.create committed before the project deletion at line 471 is never locked or cleaned up; re-read and serialize membership immediately before committing the project deletion, or reject and retry when the child set changes.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/project/ProjectService.ts around line 396:
`deleteProject` can soft-delete a project while leaving a newly created live V2 thread attached to it, along with that thread’s run/provider-session resources. Because `projectThreads` comes from the one-time shell snapshot at line 389, a `thread.create` committed before the project deletion at line 471 is never locked or cleaned up; re-read and serialize membership immediately before committing the project deletion, or reject and retry when the child set changes.
| Effect.fn("environment.projects.mutate")(function* (args) { | ||
| yield* annotateEnvironmentRequest(args.endpoint.name); | ||
| yield* requireEnvironmentScope(AuthOrchestrationOperateScope); | ||
| const operation = projectMutationOperation(projects, args.payload); |
There was a problem hiding this comment.
🟡 Medium project/http.ts:55
Deleting a project through this HTTP mutate endpoint leaves any tracked background clone running, so it can finish into the removed project's destination and retain its unfinished checkout. Unlike the WebSocket mutation path, projectMutationOperation does not trigger ProjectCloneTracker.discard after a successful project.delete; invoke that cleanup for HTTP deletes as well.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/project/http.ts around line 55:
Deleting a project through this HTTP `mutate` endpoint leaves any tracked background clone running, so it can finish into the removed project's destination and retain its unfinished checkout. Unlike the WebSocket mutation path, `projectMutationOperation` does not trigger `ProjectCloneTracker.discard` after a successful `project.delete`; invoke that cleanup for HTTP deletes as well.
| Effect.mapError(() => new OrchestratorProjectionError({ threadId: command.threadId })), | ||
| ); | ||
| if (request === undefined || request.status !== "pending") { | ||
| return yield* new OrchestratorD |
There was a problem hiding this comment.
🟠 High orchestration-v2/Orchestrator.ts:8449
A failed, interrupted, or rolled-back delegated wake is marked delivered, so its claimed tasks are not included in pendingTaskIds and no successor wake is reserved; the parent therefore never receives the child result. Only completed should settle these tasks as delivered; all other terminal statuses must return them to pending.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/Orchestrator.ts around line 8449:
A failed, interrupted, or rolled-back delegated wake is marked `delivered`, so its claimed tasks are not included in `pendingTaskIds` and no successor wake is reserved; the parent therefore never receives the child result. Only `completed` should settle these tasks as delivered; all other terminal statuses must return them to `pending`.
| }), | ||
| Schema.Struct({ | ||
| type: Schema.Literal("queued-run.edit"), | ||
| context: Schema.optional(OrchestrationMessageContext), |
There was a problem hiding this comment.
🟠 High src/orchestrationV2.ts:2553
queued-run.edit cannot clear a queued message's context, so removing the final context-linked attachment leaves stale context that is later delivered with the message. Because context is optional here, the mobile edit flow omits it when the replacement is empty, and the dispatcher interprets omission as “leave unchanged.” Use a nullable replacement (or another explicit clear operation) to distinguish clearing context from leaving it unchanged.
Also found in 1 other location(s)
apps/server/src/orchestration-v2/Orchestrator.ts:7121
Queued edits cannot clear message context.
queued-run.edittreats an omittedcommand.contextas “leave the old context”, butresolveQueuedEditPayloaddeliberately returnscontext: undefinedafter attachment-linked records are removed (mobile statequeued-run-edit.tslines 107-141). The client then omits the field (editQueuedRunonly includes it when truthy), so this payload retains the previous context, including references to attachments just removed. Persist an explicit context replacement/clear operation (or clear it whenever an attachment replacement has no context) so edited queued messages do not later execute with stale context.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/contracts/src/orchestrationV2.ts around line 2553:
`queued-run.edit` cannot clear a queued message's context, so removing the final context-linked attachment leaves stale context that is later delivered with the message. Because `context` is optional here, the mobile edit flow omits it when the replacement is empty, and the dispatcher interprets omission as “leave unchanged.” Use a nullable replacement (or another explicit clear operation) to distinguish clearing context from leaving it unchanged.
Also found in 1 other location(s):
- apps/server/src/orchestration-v2/Orchestrator.ts:7121 -- Queued edits cannot clear message context. `queued-run.edit` treats an omitted `command.context` as “leave the old context”, but `resolveQueuedEditPayload` deliberately returns `context: undefined` after attachment-linked records are removed (mobile state `queued-run-edit.ts` lines 107-141). The client then omits the field (`editQueuedRun` only includes it when truthy), so this payload retains the previous context, including references to attachments just removed. Persist an explicit context replacement/clear operation (or clear it whenever an attachment replacement has no context) so edited queued messages do not later execute with stale context.
| Effect.mapError(() => new OrchestratorProjectionError({ threadId: command.threadId })), | ||
| ); | ||
| if (request === undefined || request.status !== "pending") { | ||
| return yield* new OrchestratorD |
There was a problem hiding this comment.
🟠 High orchestration-v2/Orchestrator.ts:9061
A transient failure in handleTerminalRun permanently drops terminal processing: if finalizeAppOwnedSubagent or queue promotion fails, the child result and wake are never applied until a server restart. The same loss occurs when the startup subagent-results or delegated-completions recovery scan fails, because those results predate the live subscription cursor. Retry failed terminal handling and recovery work with durable or otherwise retained state instead of only logging the error in this catch.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/Orchestrator.ts around line 9061:
A transient failure in `handleTerminalRun` permanently drops terminal processing: if `finalizeAppOwnedSubagent` or queue promotion fails, the child result and wake are never applied until a server restart. The same loss occurs when the startup `subagent-results` or `delegated-completions` recovery scan fails, because those results predate the live subscription cursor. Retry failed terminal handling and recovery work with durable or otherwise retained state instead of only logging the error in this catch.
| // request's connection and interrupt the fiber). | ||
| return yield* Effect.uninterruptibleMask((restore) => | ||
| Effect.gen(function* () { | ||
| const worktree = yield* restore( |
There was a problem hiding this comment.
🟡 Medium mcp/WorktreeMcpService.ts:247
Cancelling during createWorktree can leave the newly created branch and worktree on disk without a thread binding. restore(...) remains interruptible until createWorktree returns, but the rollback is only established afterward in recheckAndBind; make the post-add acquisition path uninterruptible or register cleanup before acquisition can be cancelled.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/mcp/WorktreeMcpService.ts around line 247:
Cancelling during `createWorktree` can leave the newly created branch and worktree on disk without a thread binding. `restore(...)` remains interruptible until `createWorktree` returns, but the rollback is only established afterward in `recheckAndBind`; make the post-add acquisition path uninterruptible or register cleanup before acquisition can be cancelled.
| recordApproval: (input) => { | ||
| const grants = scopeFor(input); | ||
| if (input.kind === "command") { | ||
| grants.execute = true; |
There was a problem hiding this comment.
🔴 Critical acp/AcpClientPolicy.ts:327
Accepting any command permission sets grants.execute = true, so every later client-mediated terminal/create in that scope is authorized regardless of the command the user approved. A provider can get approval for one benign command and then run an unrelated command with the T3 server's privileges; the grant must be bound to the approved command (or terminal execution must require a separate matching approval).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/provider/acp/AcpClientPolicy.ts around line 327:
Accepting any `command` permission sets `grants.execute = true`, so every later client-mediated `terminal/create` in that scope is authorized regardless of the command the user approved. A provider can get approval for one benign command and then run an unrelated command with the T3 server's privileges; the grant must be bound to the approved command (or terminal execution must require a separate matching approval).
| const sourcePlan = | ||
| command.sourcePlanRef === undefined | ||
| ? null | ||
| : (sourcePlanProjection?.plans.find( |
There was a problem hiding this comment.
🟠 High orchestration-v2/Orchestrator.ts:4275
Every message.dispatch with a valid sourcePlanRef is rejected as Proposed plan ... does not exist, so plan-based dispatch cannot run. getProjectionWithPendingEvents uses readCommandProjection, which constructs projections with plans: []; load the plans into that projection before this lookup.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/Orchestrator.ts around line 4275:
Every `message.dispatch` with a valid `sourcePlanRef` is rejected as `Proposed plan ... does not exist`, so plan-based dispatch cannot run. `getProjectionWithPendingEvents` uses `readCommandProjection`, which constructs projections with `plans: []`; load the plans into that projection before this lookup.
| return { action: "decline" } as const; | ||
| } | ||
| const url = normalizeAcpRegistryWebUrl(request.url); | ||
| const elicitationId = boundedOpaqueValue(request.elicitationId, MAX_ID_LENGTH); |
There was a problem hiding this comment.
🟠 High acp/AcpRegistryProbe.ts:382
URL-authentication requests with elicitation IDs from 129–256 characters are declined, so the probe fails instead of presenting the browser-login action. Both probeAcpRegistryConfiguration and makeAcpRegistryManagementRuntime use boundedOpaqueValue(..., MAX_ID_LENGTH) (128), although AcpRegistryUrlAuthAction and the ACP adapter support IDs up to 256; validate these IDs against the action contract's 256-character limit.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/provider/acp/AcpRegistryProbe.ts around line 382:
URL-authentication requests with elicitation IDs from 129–256 characters are declined, so the probe fails instead of presenting the browser-login action. Both `probeAcpRegistryConfiguration` and `makeAcpRegistryManagementRuntime` use `boundedOpaqueValue(..., MAX_ID_LENGTH)` (128), although `AcpRegistryUrlAuthAction` and the ACP adapter support IDs up to 256; validate these IDs against the action contract's 256-character limit.
| const remainingTaskIds = delivery.taskIds.filter((taskId) => taskId !== task.id); | ||
| const deliveryRun = completionDeliveryRun(projection, delivery); | ||
| const clearDelivery = | ||
| remainingTaskIds.length === 0 && |
There was a problem hiding this comment.
🟠 High orchestration-v2/Orchestrator.ts:1783
When the last task in a delivery is acknowledged or disposed after its delivery run has completed, parentRun.delegatedCompletion.delivery remains non-null with an empty taskIds, so later terminal child completions cannot reserve a successor wake and never wake the parent. clearDelivery only clears an absent or queued delivery run; clear or advance exhausted terminal deliveries as well so every child terminal state satisfies completionWake: "always".
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/Orchestrator.ts around line 1783:
When the last task in a delivery is acknowledged or disposed after its delivery run has completed, `parentRun.delegatedCompletion.delivery` remains non-null with an empty `taskIds`, so later terminal child completions cannot reserve a successor wake and never wake the parent. `clearDelivery` only clears an absent or `queued` delivery run; clear or advance exhausted terminal deliveries as well so every child terminal state satisfies `completionWake: "always"`.
|
Closing this stale V2 proposal after retesting main at 37de6cb. Switching queued-message editors still loses an unsaved draft; the focused reproduction and short before/after videos are preserved in #15911, which was withdrawn because active #15612 already includes the same guard. Track the remaining draft-loss fix in #15612. The additional queue-navigation shortcuts and presentation changes here are separate product proposals, not part of that bug fix, and are not being refiled without maintainer direction. |
Extends #12599: Option/Alt+Up walks backward through queued messages, and Option/Alt+Down walks forward to the original composer draft. Unsaved text, attachment, and context changes require Save or Cancel before switching, including when clicking another row's pencil.
The configured send shortcut saves the edited message in place and restores the original draft. Escape discards the edit and restores that draft. Suggestions and attachment previews close first; holding Escape does not also discard the edit. Shift+Enter retains its newline behavior. Navigation shortcuts are configurable. Image thumbnails stay on the left, with a one-pixel vertical adjustment to align visually with the message text.
Targets V2 (
t3code/codex-turn-mapping), based on1f2f91ded4. Parent #12599 is already merged into V2.Validation
c808dedbf2: real Linux Chromium verification at normal width, a narrow window, and 125% zoom; backward/forward navigation, dirty-edit protection, Escape, Ctrl+Enter save, and draft restoration checked again. Targeted lint, formatting, and diff checks pass. Full CI passed on this head (Check, Test, all three server test shards, Rust, and Release Smoke). Macroscope correctness and UI consistency passed with no findings; the unchanged Effect check was skipped. Macroscope requires human review for the workflow and default-keybinding changes.024abdc43a, before the one-line thumbnail adjustment: 259 focused tests in six files, web typecheck, and full CI passed. Macroscope correctness, Effect conventions, and UI consistency passed with no findings. Approvability requires human review because this changes the queue-editing workflow and default shortcuts.Native queue sheet redesign (
74ec2ceae2, iOS and Android)The native "Queued" sheet was a flat one-line list with small handles and a weak "Resume queue" pill. It now uses a rounded grouped card. Each row shows two lines of text, a status line (up next, position, attachment count, or "Editing in composer"), a 44 px image thumbnail and a quieter Steer pill. A held queue shows a "Queue paused" banner with an inline Resume, and a hint below the list explains tap, hold and swipe. Behavior is unchanged apart from layout. Checked on a real queue against the isolated server (iPhone 17 Pro sim on iOS 26.5, Android 16 emulator): the editing state, reorder handles and the paused banner all render, and Resume drained the held queue. Mobile typecheck and the queue presentation tests pass.
c808ded)74ec2ce)The mobile recordings below were taken at
c808ded, before this redesign. The unsaved-edit loss on mobile is still open.Verification at
c808dedbf2(Mac, isolated state, real Codex turns)Tested head
c808dedbf2f23f567bd7bc116047d0ce6c034b5con V2 without rebasing. Each client used a queue of four real messages behind an active turn: a short one, a long multiline one, one with an image, and one more. Queue state was checked against the server's database after each save, cancel, reorder and removal.Mobile unsaved-edit loss (pre-existing, not from this PR): on both iOS and Android, open a queued message, type
X, open another queued entry, then return. The field shows the saved text again with no Save/Cancel prompt, and theXis lost. The native queue sheet has no dirty-edit guard; this PR changes only the shared web/desktop composer. Mobile needs a separate fix.Other notes (none caused by this PR):
mobile-native-client.ts ensure androidfails for emulators because it validates the serial and Expo expects the AVD name.libfbjni.soneeds__cxa_init_primary_exception); it runs when built with NDK 28.2.<body>, so the next shortcut does nothing until the composer is clicked.Mac web demonstration (82 s, normal speed)
MP4: Option+Up to the first entry, blocked switches with the toast, Enter save, reopen to show it persisted, Escape cancel with draft restored, suggestions closed by the first Escape, forward navigation to the draft, and ⌘Enter save during an active turn. Idle waits are cut; each action plays at recorded speed.
iOS simulator: native queue workflow (75 s)
MP4: held queue, then the
Xedit is lost after switching, Update persists, Cancel restores the draft, Move up, Remove, and Resume drains.Android emulator: native queue workflow (119 s)
MP4: the same sequence as iOS on Android, now connected: successful Save, reorder, removal and draining are verified (these were blocked in the earlier Android pass). The
Xloss reproduces.Thumbnail alignment (before/after)
Earlier Android evidence from the 2026-09-21 pass, which first reproduced the mobile edit loss: before switching · after returning · real-time recording
Full test report · Previous report
Scope: shared web/desktop composer only. Not tested: Electron shell, Windows/Linux, IME input, relay/tunnel. Native mobile keyboard shortcuts are not implemented. Mobile unsaved-edit loss remains unresolved and needs a separate native fix. No source changes in this verification pass.
Verification pass: Claude Opus 5.5 via Claude Code (T3 Code). Original implementation: GPT-6-Astra, Codex.